Skip to content

Support for non-serializable component parameters #120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 57 commits into from
Feb 2, 2023
Merged

Support for non-serializable component parameters #120

merged 57 commits into from
Feb 2, 2023

Conversation

@Archmonger
Copy link
Contributor

@Archmonger Archmonger commented Jan 10, 2023 •
edited
Loading

Description

Functional implementation of database-backed component params

Changelog

  • Database backed component parameters
    • The component template tag now supports both positional and keyword arguments.
    • The component template tag now supports non-serializable arguments.
    • It is now mandatory to run manage.py migrate after installing IDOM
    • IDOM_WS_MAX_RECONNECT_TIMEOUT has been renamed to IDOM_RECONNECT_MAX and is now used as the default timeout for component parameters
  • Bumped the minimum IDOM version to 0.43.0
    • idom.backend.hooks support.
    • django_idom.types.IdomWebsocket has been renamed to Connection and modified to fit the new schema
    • view_to_component utility will now automatically use del_html_head_body_transform
    • Set IDOM_DEBUG_MODE to settings.py:DEBUG
  • The react client is now set to production rather than development
  • Make a harmless utils public but undocumented
  • Documentation for django_query_postprocessor

Checklist:

Please update this checklist as you complete each item:

  • Tests have been included for all bug fixes or added functionality.
  • The changelog has been updated with any significant changes, if necessary.
  • GitHub Issues which may be closed by this PR have been linked.

@Archmonger
Copy link
Contributor Author

@rmorshea This is fully functional but currently has no eviction/expiration strategy implemented. Let me know what you think.

@Archmonger Archmonger linked an issue Jan 10, 2023 that may be closed by this pull request
Databased-backed component args/kwargs #119
Closed
@rmorshea
Copy link
Contributor

Will take a look tomorrow

rmorshea
rmorshea reviewed Jan 10, 2023
View reviewed changes
@rmorshea
Copy link
Contributor

Overall LGTM. Perhaps we could add a test which passes a parameter to a component that isn't JSON serializable?

rmorshea
rmorshea reviewed Jan 11, 2023
View reviewed changes
@Archmonger
Copy link
Contributor Author

Archmonger commented Jan 11, 2023 •
edited
Loading

The reconnect variable has been changed to better describe the new behavior. Also, the rename helps in preparation for reactive-python/reactpy#525

In the future, I hope to introduce a IDOM_RECONNECT_INTERVAL setting as well.

@Archmonger Archmonger changed the title Database-backed component params Support for non-serializable component parameters Jan 11, 2023
@Archmonger
Copy link
Contributor Author

Changelog in the PR's description has been updated.

@Archmonger
Copy link
Contributor Author

I've added tests for the component parameter cleanup logic

@Archmonger
Copy link
Contributor Author

@rmorshea This has security fixes, so I'd like to get this out soon if possible.

@Archmonger Archmonger changed the title v3.0.0: Support for non-serializable component parameters Support for non-serializable component parameters Jan 28, 2023
@Archmonger
Copy link
Contributor Author

I removed the version bump from this PR.

I will create a follow-up PR that bumps IDOM to 1.0.0 and Django-IDOM to 3.0.0.

@Archmonger Archmonger requested a review from rmorshea February 1, 2023 05:45
@rmorshea
Copy link
Contributor

rmorshea commented Feb 1, 2023

Will review tomorrow.

rmorshea
rmorshea reviewed Feb 2, 2023
View reviewed changes
Copy link
Contributor

@rmorshea rmorshea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally approve, just few minor nits/comments

@Archmonger
Copy link
Contributor Author

Let me know if we're okay with the approach on the 3 unresolved comments

rmorshea
rmorshea approved these changes Feb 2, 2023
View reviewed changes
Copy link
Contributor

@rmorshea rmorshea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think everything looks good to me.

@Archmonger Archmonger merged commit da083ac into reactive-python:main Feb 2, 2023
@Archmonger Archmonger deleted the database-backed-kwargs branch February 2, 2023 06:33
@Archmonger Archmonger restored the database-backed-kwargs branch February 9, 2023 03:37
@Archmonger Archmonger deleted the database-backed-kwargs branch February 21, 2023 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rmorshea rmorshea rmorshea approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Databased-backed component args/kwargs Set React Client to PRODUCTION by default Set IDOM_DEBUG_MODE to settings.py:DEBUG

2 participants

@Archmonger @rmorshea